Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-2939: Update PULL_REQUEST_TEMPLATE #2940

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

wgtmac
Copy link
Member

@wgtmac wgtmac commented Jul 3, 2024

Rationale for this change

The original PR template is a little bit out of date, especially after moving from JIRA to Github issue.

What changes are included in this PR?

Update PULL_REQUEST_TEMPLATE to remove unnecessary check boxes and follow what Apache Arrow does.

Are these changes tested?

This is not a code change.

Are there any user-facing changes?

No.

Closes #2939

@wgtmac
Copy link
Member Author

wgtmac commented Jul 3, 2024

@rok @gszadovszky @Fokko @julienledem Would you mind taking a look?

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up @wgtmac

I left a few comments. My personal preference is that these PR templates should be as short and concise as possible. I think that our old template is already a bit on the long side. My feeling is that people don't read them when they become too long. That's why I'm a bit of a nit-pick on the wording, sorry for that. I also suggested removing the markdown from the comments, since they won't render anyway.

Let me know what you think!

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gszadovszky gszadovszky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wgtmac for doing this. I agree with @Fokko's comments, otherwise LGTM.

@wgtmac
Copy link
Member Author

wgtmac commented Jul 3, 2024

Thanks for the suggestion! I've simplified the wording and removed unimportant comments. PTAL. @Fokko @gszadovszky

@wgtmac
Copy link
Member Author

wgtmac commented Jul 8, 2024

@Fokko Gentle ping :)

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @wgtmac Thanks for fixing this 👍

@Fokko Fokko merged commit 0876679 into apache:master Jul 8, 2024
9 checks passed
@wgtmac
Copy link
Member Author

wgtmac commented Jul 8, 2024

Thanks @Fokko for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update PULL_REQUEST_TEMPLATE
3 participants